Consider actual indentation of previous line for smart indent #8307
Conversation
Triton171
changed the title
pascalkuthe
added
E-medium
Triton171
force-pushed
the
relative-indent-queries
branch
from
b42627e
to
95bf0a7
Compare
|
After I manually closed #8124, I realised it shouldn't have been because plain text is still an issue - this PR would also have closed that, given that it works great with plain text. Fantastic contribution, looking forward to when this is merged! It feels great to use. |
Triton171
force-pushed
the
relative-indent-queries
branch
2 times, most recently
from
1834d8d
to
0b7abe3
Compare
|
I've tested the changes a bit and it seems to work fairly well. There are a few situations in which the new heuristic is worse than the old one, for example: int main() {
int var = complicated_expression_spanning_multiple_lines
+ which_is_aligned;
// This is the indentation level of a newline here with the new heuristic
// This is the expected indentation level (which the old heuristic correctly guesses)
}This can only happen if the line above is not indented as expected and this error does not also apply to the new line. This means that it tends to happen in languages where our current indent queries are not very reliable. Since in these case the new heuristic is a lot more resilient overall, I think this is fine for now. In the future, we could think about using the syntax tree to decide which line should be used as a baseline for comparing the indent (in my implementation, this is just the previous non-blank line). It probably makes sense to do this in another PR though, since we might have more examples where my implementation is suboptimal by then. Also, these issues can often be solved by adding missing |
Triton171
force-pushed
the
relative-indent-queries
branch
from
1808fc5
to
d242daa
Compare
|
Works great for me! |
i.e. also take into account the indentation of a previous line when computing the indentation for a new line.
Add tests to ensure that relative & absolute indent computation are consistent.
Since the tree-sitter grammar is not very good at parsing function calls while they're being written, this is not yet super useful. However, it prevents the new `hybrid` indent heuristic from choosing these lines as a baseline, making it more robust.
Increase hybrid indent heuristic attempt limit to 4. Clarify the fallback logic in indent heuristic docs.
Triton171
force-pushed
the
relative-indent-queries
branch
from
d242daa
to
bf311ec
Compare
|
Thanks for the reminder that this PR still exists :D. I rebased it onto the latest master and added 2 alignment queries for C, mainly since they aid the new hybrid indent heuristic in choosing a suitable baseline. Since I've daily-driven this for the past few months at work (doing C++ development), I'm reasonably confident that the code shouldn't have any major bugs. Some other languages might also still have missing alignment queries which could cause issues with the new indent heuristic. I think these should be rare, so we can just fix them whenever they are reported (there's not really a way to find them other than using Helix to edit code in the language for a significant amount of time). |
Essentially, to compute the indentation of a new line, we run an indent query both for the insertion position and for a non-empty line above. Then, we add the difference of these computed indents to the actual indentation of the line above. This makes indent queries a lot more resilient: Any error / inconsistency which applies to both lines will be canceled out, e.g.:
The main thing still missing from this PR is some way to disable relative indent queries. While this shouldn't really be necessary when using Helix, relative indent computation makes debugging indent queries more difficult. Should this be an option? Or is there a better way to do it?
I also haven't tested my changes a lot, so I appreciate it if anyone wants to try it out. I'll also use it myself for some time before marking the PR ready for merging.
The only significant downside of this change is that we now compute 2 (or in rare cases 3) indent queries when inserting a new line. The time needed for this only becomes noticable when inserting a new line with many cursors at once. If this is an issue, we could disable smart indent when inserting many (e.g. > 1000) newlines at once. For now, I think it is fine though (I haven't seen any complaints about the current indent queries being slow with many cursors either).
Fixes #6235
Fixes #7206
Fixes (partially) #6939 and #1725